Skip to content

Conversation

@ParidelPooya
Copy link
Contributor

@ParidelPooya ParidelPooya commented Dec 12, 2025

- Track finished runInChildContext in finishedAncestors
- Simplified hasFinishedAncestor that only checks finishedAncestors
- Enable map-min-successful and parallel-min-successful tests

@ParidelPooya ParidelPooya force-pushed the feat/finished-ancestors-clean branch 3 times, most recently from 9ecc198 to 4414401 Compare December 12, 2025 18:06
- Add finishedAncestors parameter to CheckpointManager constructor
- Track completed operations (SUCCEED/FAIL) in finishedAncestors set
- Update all CheckpointManager instantiation sites
- Remove obsolete ancestor completion tests
- All tests passing (725/725)
- Build successful across all packages
- Removed pendingCompletions and ancestor completion methods
- Added finishedAncestors Set for tracking completed operations
- Implemented parent mapping for ancestor traversal
- Reduce expected InvocationCompleted events from 4 to 2
- Reflects new behavior where finishedAncestors prevents redundant operations
…rsing

    - Remove parentMapping Map from CheckpointManager to reduce memory usage
    - Add getParentId helper to parse hierarchical stepIds (e.g., '1-2-3' → '1-2')
    - Move finishedAncestors marking to run-in-child-context handler for proper scoping
    - Add markAncestorFinished method to Checkpoint interface for explicit control
    - Update all mock checkpoints to include new method for test compatibility
    - Temporarily disable parallel-wait assertion while investigating timing differences
anthonyting
anthonyting previously approved these changes Dec 16, 2025
…abstraction level

- Move operation names from step level to parallel branch level using NamedParallelBranch
- Move operation names from step level to map item level using itemNamer property
- This fixes timing issues where child operations completed before parent operations were marked as finished
- Checkpoint skipping now works correctly at the proper abstraction level where ancestor checking functions properly
…lity

- Test markAncestorFinished method for adding stepIds to finished ancestors set
- Test hasFinishedAncestor method for proper ancestor hierarchy checking
- Test checkpoint skipping behavior when ancestors are finished
- Test integration with complex nested hierarchies
- Verify that only true ancestors (not siblings) trigger checkpoint skipping
- All 13 tests passing with good coverage of the new functionality
- Remove 🧪 TESTING console logs from checkpoint handlers
- Clean up debug output that was still printing during tests
- Tests now run cleanly without verbose checkpoint logging
- Add ancestor cleanup logic in checkpoint manager termination evaluation
- Use original stepId from metadata to avoid hashed stepId issues
- Clean up operations with finished ancestors in RETRY_WAITING, IDLE_NOT_AWAITED, and IDLE_AWAITED states
- Add comprehensive unit tests for ancestor cleanup functionality
- Update CompletionConfig TSDoc with race condition behavior note

Fixes memory leaks and ensures proper resource cleanup when ancestor contexts complete.
The min-successful-with-passing-threshold test was a duplicate that caused
module resolution issues in the integration tests. The fix was already
applied to the original parallel-min-successful test.
Changed config.name from 'Parallel minSuccessful' to 'Parallel minSuccessful with Passing Threshold'
to avoid Lambda function mapping conflict with the existing parallel-min-successful test.
anthonyting
anthonyting previously approved these changes Dec 16, 2025
- Replace storage.operationDataMap with storage.getAllOperationData()
- Fixes TypeScript compilation error about accessing private property
@ParidelPooya ParidelPooya marked this pull request as ready for review December 16, 2025 21:59
@ParidelPooya ParidelPooya merged commit f4ed60a into main Dec 16, 2025
11 checks passed
@ParidelPooya ParidelPooya deleted the feat/finished-ancestors-clean branch December 16, 2025 21:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants